Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: add compute units to jsonrpc parser #27466

Merged
merged 1 commit into from
Sep 1, 2022
Merged

fix: add compute units to jsonrpc parser #27466

merged 1 commit into from
Sep 1, 2022

Conversation

mschneider
Copy link
Contributor

@mschneider mschneider commented Aug 30, 2022 •
edited
Loading

Problem

The auto-complete of my editor did not show the computeUnitsConsumed field.

Tested with this example script:

import { Connection } from "@solana/web3.js";

async function main() {
  const conn = new Connection("https://testnet.rpcpool.com");
  const commitment = "confirmed";
  const version = await conn.getVersion();
  console.log({ version });
  const slot = await conn.getSlot(commitment);
  console.log({ slot });
  const block = await conn.getBlock(slot, { commitment });
  console.log({ block });
  const tx = block?.transactions[0];
  console.log("tx", tx);
}

main();

@mergify mergify bot added the community Community contribution label Aug 30, 2022
@mergify mergify bot requested a review from a team August 30, 2022 13:31
@steveluscher steveluscher added the javascript Pull requests that update Javascript code label Sep 1, 2022
@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #27466 (c50e252) into master (ada493f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #27466   +/-   ##
=======================================
  Coverage    76.7%    76.7%           
=======================================
  Files          52       52           
  Lines        2651     2651           
  Branches      364      364           
=======================================
  Hits         2035     2035           
  Misses        483      483           
  Partials      133      133

steveluscher
steveluscher reviewed Sep 1, 2022
View reviewed changes
web3.js/src/connection.ts
Comment on lines +870 to +871
/** The addresses of the accounts loaded for the transaction */
loadedAddresses?: LoadedAddresses | null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed a spot from #27065, @jstarry?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup it was missed but recently added in #27068, this addition just broke master unfortunately. In the future let's keep unrelated changes out of PR's

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha. Scroll down, @steveluscher. Sorry about that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no worries! Merge conflicts happen, and it was originally my fault for missing that field :)

@steveluscher steveluscher changed the title add compute units to jsonrpc parser fix: add compute units to jsonrpc parser Sep 1, 2022
steveluscher
steveluscher approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steveluscher steveluscher requested a review from jstarry September 1, 2022 16:33
@steveluscher steveluscher merged commit b34cab4 into solana-labs:master Sep 1, 2022
jstarry added a commit that referenced this pull request Sep 1, 2022
@jstarry
Revert "fix: add compute units to jsonrpc parser (#27466)"
97b296f 
This reverts commit b34cab4.
@jstarry jstarry mentioned this pull request Sep 1, 2022
Merged
jstarry added a commit that referenced this pull request Sep 1, 2022
@jstarry
Revert "fix: add compute units to jsonrpc parser" (#27553)
31087b8 
Revert "fix: add compute units to jsonrpc parser (#27466)"

This reverts commit b34cab4.
@jstarry jstarry mentioned this pull request Sep 1, 2022
Merged
This was referenced Sep 23, 2022
Open
Open
Open
This was referenced Oct 1, 2022
Open
Open
Open
Open
This was referenced Oct 11, 2022
Open
Open
@Balantion2020 Balantion2020 mentioned this pull request Oct 16, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Oct 25, 2022
Open
This was referenced Oct 28, 2022
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveluscher steveluscher steveluscher approved these changes

@jstarry jstarry Awaiting requested review from jstarry

Assignees
No one assigned
Labels
community Community contribution javascript Pull requests that update Javascript code
Projects
None yet
Milestone
web3.js Roadmap – September 2022
Development

Successfully merging this pull request may close these issues.
3 participants
@mschneider @steveluscher @jstarry